Skip to content

Conversation

@rlezuo1
Copy link
Contributor

@rlezuo1 rlezuo1 commented Aug 10, 2023

Add support for Flash readout protection on the STM32L4x series

Copy link
Contributor

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I can see is that the provided functions are not used anywhere, there is no test for them there is no sample, and they have no external declarations provided.
What are they for?
If they are for users, then they should be implemented as flash_ex_op, as a vendor extension (include/zephyr/drivers/flash/stm32_flash_api_extensions.h).
Is this similar work as here #52980 and here #53441 ?

@de-nordic de-nordic requested a review from duda-patryk August 17, 2023 11:17
@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Sep 4, 2023

@de-nordic yes the work is similar to the PRs you've referenced, it implements one of the features (RDP) on another ST part following the spirit of the existing code.
I agree that samples and tests are missing (as for the existing code).
Testing this requires real hardware, a sample could be provided, looks like this, wouldn't know where to add it though.

        struct flash_stm32_ex_op_rdp rdp_in = {
            .enable = true,
            .permanent = false
        };
        struct flash_stm32_ex_op_rdp rdp_out;

        if (flash_ex_op(flash, FLASH_STM32_EX_OP_RDP, (uintptr_t)&rdp_in, &rdp_out) != 0) {
            LOG_ERR("RDP level NOT set");
        } else {
            LOG_INF("RDP levels: enable=%d permanent=%d", rdp_out.enable, rdp_out.permanent);
        }

@semihalf-duda-patryk any suggestions/ideas on this one?

@erwango
Copy link
Member

erwango commented Sep 4, 2023

I agree that samples and tests are missing (as for the existing code).

Tests are available in tests/drivers/flash/stm32/src/main.c

@rlezuo1 rlezuo1 force-pushed the add_stm32l4x_rdp branch 2 times, most recently from 6b72f78 to 70fdbb7 Compare September 6, 2023 16:41
@rlezuo1 rlezuo1 requested a review from de-nordic September 7, 2023 06:53
erwango
erwango previously approved these changes Sep 12, 2023
@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Sep 18, 2023

@de-nordic I believe the change-requests you had have been addressed is there still something missing?

Add support for Flash readout protection on the STM32L4x series

Signed-off-by: Roland Lezuo <[email protected]>
@rlezuo1 rlezuo1 force-pushed the add_stm32l4x_rdp branch 2 times, most recently from 7dfabc9 to 0cfff2d Compare October 3, 2023 17:36
@de-nordic de-nordic dismissed their stale review October 3, 2023 18:00

re-review

@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Oct 12, 2023

@semihalf-duda-patryk @erwango friendly reminder

@erwango erwango added this to the v3.6.0 milestone Oct 12, 2023
Add device tests (successfully executed locally)

Signed-off-by: Roland Lezuo <[email protected]>
@rlezuo1 rlezuo1 requested a review from erwango October 12, 2023 13:25
@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Oct 12, 2023

@de-nordic friendly poke - with your approval the PR can be merged ...

@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Oct 15, 2023

@erwango friendly poke: can you merge this PR?

@erwango
Copy link
Member

erwango commented Oct 16, 2023

@erwango friendly poke: can you merge this PR?

No. We're in feature freeze for V3.5.0 stabilisation period. It will have to wait until merge window is open again.
See https://github.com/zephyrproject-rtos/zephyr/wiki/Release-Management

@rlezuo1
Copy link
Contributor Author

rlezuo1 commented Oct 16, 2023

@erwango full ack, sorry for the noise

@carlescufi carlescufi merged commit b97376d into zephyrproject-rtos:main Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants